Skip to content

Decouple DisplayServer from as much of the codebase as possible#116976

Open
akien-mga wants to merge 4 commits intogodotengine:masterfrom
akien-mga:displayserver-split-types
Open

Decouple DisplayServer from as much of the codebase as possible#116976
akien-mga wants to merge 4 commits intogodotengine:masterfrom
akien-mga:displayserver-split-types

Conversation

@akien-mga
Copy link
Member

@akien-mga akien-mga commented Mar 2, 2026

I kept commits separate from review, but the "Move" commits can be squashed before merge, and the resulting commit can be added to the .git-blame-ignore-revs.

I haven't moved all enums yet, only the ones necessary to decouple DisplayServer from the most critical headers.
For consistency with RenderingServer and AccessibilityServer, we probably want to move all enums to DisplayServerEnums anyway. That will also allow removing a few extra includes.

With these changes, an incremental rebuild after changing display_server.h on my machine changes like this:

  • Before: 00:02:37.65
  • After: 00:00:47.36 00:00:42.10

The gains are probably mainly from removing display_server.h from viewport.h and rendering_server.h (control.h was decoupled by #116839, my "before" number is from before this PR as I initially also decoupled from control.h here).

@akien-mga akien-mga added this to the 4.x milestone Mar 2, 2026
@akien-mga akien-mga requested review from a team as code owners March 2, 2026 20:11
@akien-mga akien-mga removed request for a team March 2, 2026 21:20
@akien-mga akien-mga requested review from a team and removed request for a team March 2, 2026 21:20
@akien-mga akien-mga force-pushed the displayserver-split-types branch 2 times, most recently from 60339d2 to 7556774 Compare March 2, 2026 22:26
@akien-mga akien-mga requested a review from a team as a code owner March 3, 2026 11:15
@akien-mga akien-mga force-pushed the displayserver-split-types branch from faae013 to bd4d15f Compare March 3, 2026 11:33
@akien-mga
Copy link
Member Author

I moved the remaining enums to DisplayServerEnums, which allowed to remove a few extra direct includes of display_server.h.

Things are a bit messy around MouseMode (duplicated in InputClassEnums and DisplayServerEnums) and CursorShape (duplicated in InputClassEnums, DisplayServerEnums and Control). There's also VirtualKeyboardType that's duplicated in LineEdit. Refactoring this seems out of scope of this PR and not necessarily beneficial.

To decouple display_server.h from the various tts_* headers, I also moved the TTSUtterance struct to a standalone struct so it can be forward-declared (still defined in display_server.h).

Finally, after looking so much at display_server.h, I went ahead and refactored the header to group APIs together with section headers, similar to what we have in rendering_server.h. So that last commit is a bit noisy but should not change behavior, unless I messed up the alternance of public/private/protected somewhere.

With these changes, I'm down to an incremental recompilation time after changing display_server.h of 00:00:42.10.

@akien-mga akien-mga removed the request for review from a team March 3, 2026 11:39
This will allow decoupling `display_server.h` from a number of headers in the
codebase which only require those enums and not all the DisplayServer API.
Notably decouples it from:
- AccessibilityServer
- DisplayServer
- Viewport
- Window
@akien-mga
Copy link
Member Author

I kept the commits separate for ease of review, as this kind of PR ends up affecting a lot of the codebase.

Once approved, I will force-push a squashed version similar to this: https://github.com/akien-mga/godot/commits/displayserver-split-types-squashed/

  • Move all enums to DisplayServerEnums
  • Ignore that commit in .git-blame-ignore-revs
  • Remove display_server.h includes everywhere possible
  • Refactor display_server.h with API sections (cosmetic)

@akien-mga akien-mga requested a review from bruvzg March 3, 2026 11:48
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, tested on macOS, iOS, and Windows.

visionOS fails to build due to vsnprintf, this is likely casued by some of previous header changes, and not this PR:

diff --git a/drivers/apple/os_log_logger.cpp b/drivers/apple/os_log_logger.cpp
index 50b695dfa9..86ddfa28e6 100644
--- a/drivers/apple/os_log_logger.cpp
+++ b/drivers/apple/os_log_logger.cpp
@@ -33,7 +33,8 @@
 #include "core/object/script_backtrace.h"
 #include "core/string/print_string.h"

-#include <cstdlib> // For malloc/free
+#include <cstdlib> // For malloc/free.
+#include <cstdio> // For vsnprintf.

 OsLogLogger::OsLogLogger(const char *p_subsystem) {
        const char *subsystem = p_subsystem;

@akien-mga akien-mga modified the milestones: 4.x, 4.7 Mar 3, 2026
@akien-mga akien-mga force-pushed the displayserver-split-types branch from bd4d15f to 76f4588 Compare March 3, 2026 13:58
@akien-mga
Copy link
Member Author

akien-mga commented Mar 3, 2026

I pushed the squashed history so this should be ready to merge.

The original history with all commits is here:

https://github.com/akien-mga/godot/commits/displayserver-split-types-history/

I'll make a separate PR to fixup the visionOS build.
Edit: Done: #117023.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants